Skip to content

Conversation

@caosjr
Copy link
Contributor

@caosjr caosjr commented Jul 1, 2025

PR Description

Upgrade SPI engine to support up to 8 lanes. For that, two new registers were inserted into Configuration Write Instruction, they are responsible for setting the SDI and SDO lane masks. Each bit represents a lane.
This PR also removes register 8'h3b from the SPI Engine regmap.
The testbench for this modification is here: analogdevicesinc/testbenches#240

PR Type

  • Bug fix (change that fixes an issue)
  • New feature (change that adds new functionality)
  • Breaking change (has dependencies in other repos or will cause CI to fail)
  • Documentation

PR Checklist

  • I have followed the code style guidelines
  • I have performed a self-review of changes
  • I have compiled all hdl projects and libraries affected by this PR
  • I have tested in hardware affected projects, at least on relevant boards
  • I have commented my code, at least hard-to-understand parts
  • I have signed off all commits from this PR
  • I have updated the documentation (wiki pages, ReadMe files, Copyright etc)
  • I have not introduced new Warnings/Critical Warnings on compilation
  • I have added new hdl testbenches or updated existing ones

Copy link
Contributor

@LBFFilho LBFFilho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this seems headed in a nice direction, and thanks for also doing a lot of small fixes and improvements to the existing code along the way.

About the version: is this going to be a minor version or a major version bump? I understand that the removed register was not used anywhere (it was broken even), but technically we're breaking anything that relied on it. Also changing the behavior of the SDI & SDO FIFOs.

Also, please check timing on ad4052/de10nano just to be sure if it's all good.

@caosjr caosjr force-pushed the sdo_extension_spi_engine branch 3 times, most recently from 8a2552e to 15eaefd Compare July 4, 2025 14:44
@caosjr caosjr changed the title library/spi_engine: Extend SDO support for the SPI Engine library/spi_engine: SDO Extension upgrade Jul 7, 2025
@caosjr caosjr force-pushed the sdo_extension_spi_engine branch 4 times, most recently from 4300d4c to 6d23286 Compare July 10, 2025 13:53
@caosjr caosjr force-pushed the sdo_extension_spi_engine branch from df039b8 to ce44e94 Compare August 5, 2025 19:36
@caosjr caosjr marked this pull request as ready for review August 7, 2025 12:59
@caosjr caosjr force-pushed the sdo_extension_spi_engine branch 2 times, most recently from 149396a to ab91a18 Compare August 7, 2025 15:14
@caosjr caosjr force-pushed the sdo_extension_spi_engine branch 2 times, most recently from cb4785c to 0ff6ae5 Compare August 7, 2025 15:19
@caosjr caosjr force-pushed the sdo_extension_spi_engine branch 3 times, most recently from 2fe3ffe to 5cf2987 Compare September 3, 2025 19:56
@caosjr caosjr force-pushed the sdo_extension_spi_engine branch from 5cf2987 to 3692374 Compare September 19, 2025 13:11
@dlech
Copy link
Collaborator

dlech commented Sep 19, 2025

I've tested this again today with the latest rebase. Seems to be working OK still.

So the only request from my side still remaining is bumping the SPI Engine IP version to 2.0.0.

@caosjr
Copy link
Contributor Author

caosjr commented Sep 23, 2025

I have updated the version of the SPI Engine to 2.0 and solved the issues of the tcl scripts.

caosjr and others added 19 commits September 29, 2025 12:19
* Extend SDO support to 8 (symmetrical with SDI support);
* Update SDI to use asymmetrical FIFO;
* Insert symmetrical FIFO for the SDO;
* Insert SDI lane mask configuration instruction to reg 3'b011;
* Insert SDO lane mask configuration instruction to reg 3'b100;
* Insert offload active interface for interconnect and execution;
* Remove register 8'h3b from spi engine regmap;
* Improvements on the critical paths of the execution module.

Prefetching on offload work iff all lanes are active.

Signed-off-by: Carlos Souza <[email protected]>
* Update documentation to include the changes done
for supporting more than one SDO lane.
* Update the register map;
* Insert SDI/SDO lane mask registers;
* Update Configuration Write Instruction;
* Update parameter from NUM_OF_SDI to NUM_OF_SDIO.

Signed-off-by: Carlos Souza <[email protected]>
There is a different register for SDI lane mask and SDO
lane mask;
Update the NUM_OF_SDI parameter to NUM_OF_SDIO parameter to reflect
that both SDI and SDO are symmetrical. The control over them is
through the SDI/SDO lane masks.

Inserts a ready signal for when the valid_indices
has finished its inner logic. This avoid the possible
issue where the latency of the command is smaller than
this logic. This could only happen in the FIFO mode.

Signed-off-by: Carlos Souza <[email protected]>
Update NUM_OF_SDI to NUM_OF_SDIO in the projects documentations.
The variable was updated on the spi_engine library since SDI and
SDO have symmetrical sizes.

Signed-off-by: Carlos Souza <[email protected]>
Updated the projects to use NUM_OF_SDIO parameter. It is the same
project, no functionality has changed.

Signed-off-by: Carlos Souza <[email protected]>
fix DATA_WIDTH register.

Signed-off-by: Carlos Souza <[email protected]>
It was missing the sdi_lane_mask register.
That was not affecting execution because the only
logic requiring this register is on the axi_spi_engine.v

Signed-off-by: Carlos Souza <[email protected]>
Previous code would create issues for lane masks different from
ALL_ACTIVE_LANE_MASK, due to a pipelining bug. Although the currently officially
supported behavior is only either one lane active or all lanes active, this is a
very simple change that helps us prepare in case we want to support other lanes
in the future, and removes some extra logic (a comparison and a mux), which can
improve timing.

Signed-off-by: Laez Barbosa <[email protected]>
Introduces logic to keep track of the number of elements in
the SDI FIFO. This is due to the asymmetric FIFO implementation
not including the logic for this, and instead outputting the
level of one of its internal FIFOs.

Signed-off-by: Laez Barbosa <[email protected]>
Fixes the bug related to the sdo lane config instruction, which
was high all the time and not for a single cycle. That was causing
the data_assemble module to be rebuilding the lookup table, and
consequently affecting the output of the offload mode.

Removed lane_index_d because it is not necessary anymore. Both FIFO
and OFFLOAD modes are using the valid_indices lookup table.

Signed-off-by: Carlos Souza <[email protected]>
The modifications performed in this library are not backwards compatible. So
this version is Spi Engine 2.0.

Signed-off-by: Carlos Souza <[email protected]>
Cosmetic changes and fixed ad463x project that had some issues after the
rebase.

Signed-off-by: Carlos Souza <[email protected]>
Fix g_sdo_shift_reg for that did not have a generate block. Also
improves timing for the spi_engine_execution_shiftreg_data_assemble
module, which was necessary for working on the quartus version of the
AD4052 project.

Signed-off-by: Carlos Souza <[email protected]>
Update scripts for generating the spi engine for quartus projects. Also
updated Makefile that had a missing fifo asym library for quartus.

Signed-off-by: Carlos Souza <[email protected]>
Fixes quartus scripts for ad4052 in quartus. Also adds optimization
options for timing closure of the project.
Removes wrong timing constraints for the coraz7s.

Signed-off-by: Carlos Souza <[email protected]>
Fixes almost full threshold parameter in fifo asym. Updated to 31,
as it was previously implementation.

Signed-off-by: Carlos Souza <[email protected]>
@caosjr caosjr force-pushed the sdo_extension_spi_engine branch from 0f4024a to e1fbc2e Compare September 29, 2025 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants